Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #23212; Asyncdispatch leaks under --mm:arc #24556

Merged
merged 8 commits into from
Dec 22, 2024

Conversation

nitely
Copy link
Contributor

@nitely nitely commented Dec 21, 2024

Fixes #23212

Inspired by this chronos PR

@nitely nitely marked this pull request as draft December 22, 2024 03:17
@nitely nitely marked this pull request as ready for review December 22, 2024 04:58
@nitely
Copy link
Contributor Author

nitely commented Dec 22, 2024

This PR adds an extra closure passed to addCallback, but it makes the continuation (identName) a nimcall in return, except if the async proc has FutureVar parameters. We can make an object of futureVars and pass it around to improve that case, I think. Maybe it's possible/better to move the futureVar logic to the closure iterator. That would also help to get rid of both closures like it's done in chronos, but that also requires a breaking change in addCallback.

@Araq
Copy link
Member

Araq commented Dec 22, 2024

That would also help to get rid of both closures like it's done in chronos, but that also requires a breaking change in addCallback.

Fine with me as long as there is some migration period and how to patch client code can be explained easily.

@Araq Araq merged commit f29234b into nim-lang:devel Dec 22, 2024
18 checks passed
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from f29234b

Hint: mm: orc; opt: speed; options: -d:release
178145 lines; 8.641s; 653.828MiB peakmem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asyncdispatch leaks under --mm:arc
2 participants